fix(fs): align post-write sync with Linux generic_write_sync by donjuanplatinum · Pull Request #2147 · DragonOS-Community/DragonOS

donjuanplatinum · 2026-07-19T23:58:29Z

Make VFS post-write sync opt-in and range-aware. Only inodes that explicitly support Linux generic write sync semantics now perform sync after a positive-length write.

Sync O_SYNC/O_DSYNC/S_SYNC writes over the actual write range not all the file , and keep S_SYNC as a datasync trigger when leaving full metadata sync to O_SYNC/__O_SYNC. Enable the behavior for regular files only on real filesystems and for block-device inodes.

Make VFS post-write sync opt-in and range-aware. Only inodes that explicitly support Linux generic write sync semantics now perform sync after a positive-length write. Sync O_SYNC/O_DSYNC/S_SYNC writes over the actual write range not all the file , and keep S_SYNC as a datasync trigger when leaving full metadata sync to O_SYNC/__O_SYNC. Enable the behavior for regular files only on real filesystems and for block-device inodes. Signed-off-by: Donjuanplatinum <donplat@barrensea.org>

fslongjin · 2026-07-20T13:05:52Z

@codex review

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: f4fc796d65

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Signed-off-by: Donjuanplatinum <donplat@barrensea.org>

fslongjin · 2026-07-23T17:18:39Z

@codex review

chatgpt-codex-connector · 2026-07-23T17:24:34Z

Codex Review: Didn't find any major issues. You're on a roll.

Reviewed commit: 854c4370ec

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

fslongjin · 2026-07-23T17:26:25Z


+    fn supports_post_write_sync(&self, file_type: FileType) -> bool {
+        file_type == FileType::File
+    }


maybe we should add a default impl or make a better design instead of write these duplicated code.

fslongjin

Request changes

This PR addresses a real problem: DragonOS currently applies post-write
O_SYNC/O_DSYNC handling from the common VFS write path even though Linux only
calls generic_write_sync() from selected file_operations.write_iter
implementations. Pseudo files and special devices therefore must not all be
forced through the same fsync path, and changing whole-file sync to the actual
successful write range is the right direction.

However, the current solution models a write-operation policy as a
default-false IndexNode capability. That creates a manually maintained
filesystem/wrapper whitelist, and the whitelist is already inconsistent with
Linux: OverlayFS would sync twice, while tmpfs/shmem should not run
generic_write_sync() at all. A future filesystem or transparent wrapper can
also silently lose O_SYNC semantics by forgetting one override.

There are additional durability blockers:

the post-write path bypasses File::sync_range_and_check_wb_error(), so a
synchronous write may fail to report and advance an existing writeback
EIO/ENOSPC for this open file description;
mount/superblock synchronous semantics are not included;
FAT opts in before its sync_file_range() provides the metadata and device
flush durability boundary required by Linux FAT fsync;
loop sync retains only an inode rather than the backing File, losing FUSE
file handles, OverlayFS per-open state, credentials, and the file wb_err
cursor; it also expands a single backing-file fsync into sync_fs(true).

The new IoGuard around loop sync is useful and should be retained, but it does
not fix the backing object/lifetime issue.

The current Dunitest result also has one directly related failure:
FuseExtended.SharedWritableMmapOSyncWriteback still expects the old whole-file
writeback count. The range-aware behavior should be kept, but the test needs to
assert that the out-of-range mmap-dirty page remains dirty until a later
fsync/msync, and that the O_SYNC pwrite produces exactly one lower fsync.

Please move this policy toward the selected open-file/write operation (or at
least an explicit per-open Generic / HandledByWrite / None policy), fix the
durability and stacked-filesystem issues below, and add/update focused
regression coverage before merging.

fslongjin · 2026-07-25T06:53:26Z

+    ///
+    /// This is intentionally opt-in: many pseudo files are reported as regular
+    /// files but their Linux write paths do not perform generic write syncing.
+    fn supports_post_write_sync(&self, _file_type: FileType) -> bool {


[P1] Model this as a write-operation policy, not a default-false inode capability

Linux does not decide whether to run generic_write_sync() from an inode
property. The concrete file_operations.write_iter either invokes the helper,
forwards IOCB_SYNC/IOCB_DSYNC to another File, or implements unrelated write
semantics.

Making this a default-false IndexNode method creates a silent durability
failure mode: every new disk filesystem and every transparent wrapper must
remember to opt in or forward this method, otherwise O_SYNC/O_DSYNC writes
still return success without syncing. The OverlayFS and tmpfs overrides in
this patch already show that the boolean is also too weak to distinguish
"generic sync", "handled by the delegated write", and "no generic sync".

Please make the selected open-file/write operation own this decision. A
bounded intermediate design would cache an explicit per-open policy such as
Generic / HandledByWrite / None in File. That avoids a full VFS rewrite while
making the responsibility explicit and removing the per-write wrapper
dispatch.

fslongjin · 2026-07-25T06:53:26Z

+            let end = start.saturating_add(written_len).saturating_sub(1);
+            if need_metadata_sync {
+                // O_SYNC: 完整同步（数据 + 元数据）
+                self.inode


[P1] Preserve the open-file writeback error contract here

Calling inode.sync_file_range() directly bypasses the wb_error_seq check that
File::sync_range_and_check_wb_error() already performs. If asynchronous
writeback recorded EIO or ENOSPC after this File was opened, and this specific
range writeback now succeeds, an O_SYNC/O_DSYNC write can incorrectly return
the byte count without reporting or advancing that error for this open file
description.

Linux generic_write_sync() reaches vfs_fsync_range(), and
file_write_and_wait_range() always calls file_check_and_advance_wb_err() after
the range writeback. Please route this through the existing File helper, for
example:

let datasync = !need_metadata_sync; self.sync_range_and_check_wb_error(start, end, datasync)?;

Advancing the file offset before a later sync error is already consistent with
Linux and should not be rolled back.

fslongjin · 2026-07-25T06:53:26Z

        flags: FileFlags,
        inode_flags: InodeFlags,
    ) -> Result<(), SystemError> {
+        if written_len == 0 || !self.inode.supports_post_write_sync(file_type) {


[P1] Include superblock synchronous semantics before this early return

The later inode_sync value only checks Metadata.flags::S_SYNC.
MountFSInode::metadata() does not fold MountFlags::SYNCHRONOUS into that field,
so a normal write on a mount -o sync filesystem still skips post-write sync.
Linux iocb_is_dsync() uses IS_SYNC(inode), which includes both inode S_SYNC and
SB_SYNCHRONOUS.

Please include the mount/superblock synchronous state in the trigger. Also,
test the cheap sync flags first and only query the filesystem policy when a
positive-length write actually requires synchronization. In the current
order, every ordinary asynchronous write performs this virtual call, and a
MountFSInode performs another delegated virtual call, even though no sync is
needed.

fslongjin · 2026-07-25T06:53:26Z

        Some(self.fs())
    }

+    fn supports_post_write_sync(&self, file_type: FileType) -> bool {


[P1] Do not run a second generic sync at the OverlayFS layer

OverlayFS write_at() calls backing_file.pwrite() with the backing File's
O_SYNC/O_DSYNC flags preserved. The backing write therefore already performs
the required post-write synchronization. Returning to the outer File and
opting OvlInode into this helper syncs the same range a second time.

Linux ovl_write_iter() forwards IOCB_SYNC/IOCB_DSYNC to the real file and does
not call generic_write_sync() again after the delegated write. Besides the
extra I/O, the second fsync introduces an error point that Linux does not have:
the delegated write and its sync may succeed, but the outer duplicate sync can
still make the syscall fail.

Please remove this override and let the backing File own the synchronous-write
boundary.

fslongjin · 2026-07-25T06:53:26Z

        Some(self.fs())
    }

+    fn supports_post_write_sync(&self, file_type: FileType) -> bool {


[P1] tmpfs/shmem does not use generic_write_sync()

Linux 6.6 shmem_file_write_iter() returns directly after
generic_perform_write() and does not call generic_write_sync(); its fsync
operation is noop_fsync. Opting tmpfs into this path makes O_SYNC/O_DSYNC
writes perform page-cache range writeback and potentially return errors that
Linux tmpfs does not produce from the write.

Please remove this override. RamFS and tmpfs should not be classified together
just because both are memory-backed: Linux RamFS uses generic_file_write_iter,
while shmem/tmpfs has its own write_iter semantics.

fslongjin · 2026-07-25T06:53:26Z

        Some(self.fs())
    }

+    fn supports_post_write_sync(&self, file_type: FileType) -> bool {


[P1] Do not opt FAT in before sync_file_range provides a durability boundary

The current FAT sync_file_range() only writes back the selected page-cache
range. It ignores datasync, does not ensure the required FAT chain/directory
entry metadata is committed, and does not issue gendisk.sync() to flush a
volatile device cache. An extending O_SYNC write can therefore return success
while the data or the metadata needed to reach it is still not power-loss
durable.

Linux fat_file_fsync() runs __generic_file_fsync(), synchronizes the FAT
mapping buffers, and finally issues blkdev_issue_flush(). Please first
implement the corresponding file-level range fsync contract, including error
waiting, required metadata, and the final device durability barrier, and only
then opt FAT into generic post-write sync. Adding only a mechanical flush at
the end would not be sufficient if the FAT metadata has not been submitted.

fslongjin · 2026-07-25T06:53:26Z

+        _datasync: bool,
+        _data: MutexGuard<FilePrivateData>,
+    ) -> Result<(), SystemError> {
+        <Self as BlockDevice>::sync(self)


[P1] Retain and fsync the backing File, not only its inode

LOOP_SET_FD currently stores only file.inode(), so after the setup fd is
closed the loop device has lost the backing open-file description and its
FilePrivateData. This is not sufficient for FUSE (file handle and open state),
OverlayFS (the active per-open backing file), credentials, or the file-level
writeback error cursor.

The new sync_file() activates BlockDevice::sync(), which calls inode.sync()
followed by inode.fs().sync_fs(true). That can either miss the actual backing
open file or expand one loop flush into a whole-filesystem sync with unrelated
writeback and duplicate device flushes. Linux loop retains lo_backing_file and
lo_req_flush() calls vfs_fsync() on that File.

Please retain an Arc for the backing object, use its file-level sync
path, and release or replace it only after in-flight I/O is drained. The new
IoGuard is useful for that transition and should remain, but it cannot recover
the open-file context once only an inode was stored. The read/write paths
should use the same retained FilePrivateData as well.

Signed-off-by: Donjuanplatinum <donplat@barrensea.org>

github-actions Bot added the Bug fix A bug is fixed in this pull request label Jul 19, 2026

chatgpt-codex-connector Bot reviewed Jul 20, 2026

View reviewed changes

Comment thread kernel/src/driver/block/loop_device/loop_device.rs

add IOGuard for loopdevice sync

854c437

Signed-off-by: Donjuanplatinum <donplat@barrensea.org>

fslongjin reviewed Jul 23, 2026

View reviewed changes

fslongjin requested changes Jul 25, 2026

View reviewed changes

change maybe_sync_after_write

6792521

Signed-off-by: Donjuanplatinum <donplat@barrensea.org>

donjuanplatinum requested a review from fslongjin July 25, 2026 16:44

Uh oh!

Uh oh!

Conversation

donjuanplatinum commented Jul 19, 2026

Uh oh!

fslongjin commented Jul 20, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

fslongjin commented Jul 23, 2026

Uh oh!

chatgpt-codex-connector Bot commented Jul 23, 2026

Uh oh!

fslongjin Jul 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fslongjin left a comment

Choose a reason for hiding this comment

Uh oh!

fslongjin Jul 25, 2026

Choose a reason for hiding this comment

Uh oh!

fslongjin Jul 25, 2026

Choose a reason for hiding this comment

Uh oh!

fslongjin Jul 25, 2026

Choose a reason for hiding this comment

Uh oh!

fslongjin Jul 25, 2026

Choose a reason for hiding this comment

Uh oh!

fslongjin Jul 25, 2026

Choose a reason for hiding this comment

Uh oh!

fslongjin Jul 25, 2026

Choose a reason for hiding this comment

Uh oh!

fslongjin Jul 25, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

fslongjin Jul 23, 2026 •

edited

Loading